[SPARK-21708][BUILD] Migrate build to sbt 1.x by LorenzoMartini · Pull Request #759 · palantir/spark

LorenzoMartini · 2021-04-19T19:45:46Z

Original PR description

Migrate sbt-launcher URL to download one for sbt 1.x.
Update plugins versions where required by sbt update.
Change sbt version to be used to latest released at the moment, 1.3.13
Adjust build settings according to plugins and sbt changes.

Migration to sbt 1.x:

enhances dev experience in development
updates build plugins to bring there new features/to fix bugs in them
enhances build performance on sbt side
eases movement to Scala 3 / dotty

No.

All existing tests passed, both on Jenkins and via Github Actions, also manually for Scala 2.13 profile.

Closes apache#29286 from gemelen/feature/sbt-1.x.

Authored-by: Denis Pyshev [email protected]
Signed-off-by: Dongjoon Hyun [email protected]

Upstream SPARK-XXXXX ticket and PR link (if not applicable, explain)

[SPARK-21708][BUILD] Migrate build to sbt 1.x
Commit: apache@6daa2ae
PR: apache#29286

What changes were proposed in this pull request?

Bump sbt version to 1.x.

There are a few non-trivial changes related to versions.
First of all the original PR was introduced on top of spark 3.1 so we had to adapt a few things for it to work on top of spark 3.0

[Important] The sbt bump introduces a mima bump and that introduces some binary breaks. If comparing the binary breaks with version 2.4 of spark, there are 200+ listed breaks. However comparing with 3.0, we get away with the breaks that are also added as excludes in the original PR (MimaExcludes.scala contains them) and only 3 additional breaks. According to what discussed in this comment from upstream it is correct for us to compare with version 3.0 and the upstream commit included this change here only because they forgot to do it after releasing 3.0. You can see they encountered the same problem upstream with this bump, from this comment.
The breaks are listed under v30excludes instead of the upstream's v31excludes because current version is 3.0.0
[Workaround] The bump broke sbt unidoc. It attempts to generate docs for generated classes and fails with NoClassDef errors. Since we don't care about generated unidocs from sbt build, we can exclude the unidoc task for now.
[Small added detail] We have to avoid loading sbt/build from cache in circle builds because circle cached the existing file that points at old sbt location (bintray).
[Important note] I reduced circle parallelism from 12 to 8 for scala tests because there is an issue where too many parallel tests might interact one with the other and that causes the task to flake immensely. It seems like a lower parallelism reduces the chances of flaking at the expense of bringing the test time up from around 33 minutes to 42 minutes)

Why are the changes needed?

The SBT bump is needed because:

sbt 0.x (current) is only available in bintray, that is gonna shutdown in a matter of days
sbt 0.x introduces some test dependency on scala module 2.10 which is an issue when bumping avro version as 2.12 is needed.

Does this PR introduce any user-facing change?

No

LorenzoMartini · 2021-04-19T19:49:29Z

.circleci/config.yml

This had to be removed because it picks up older build/sbt and doesn't get the updated versions

LorenzoMartini · 2021-04-19T19:50:37Z

dev/run-tests.py

We don't need unidoc and this broke attempting to generate docs for autogenerated files. Didn't want to invest too much into this but can try if needed

LorenzoMartini · 2021-04-19T19:51:58Z

project/MimaBuild.scala

This file is cleanly picked from upstream. It seems like they forgot to do this bump and added it with this PR out of convenience (context: apache#29286 (comment) and apache#22977 (comment))

If we keep the previous to 2.4.0 the number of binary breaks is 200+ so this is the best option anyways

LorenzoMartini · 2021-04-19T19:52:32Z

project/MimaExcludes.scala

These last 3 excludes had to be added in our fork only. It should be ok

LorenzoMartini · 2021-04-19T19:53:30Z

project/SparkBuild.scala

Sbt got stricter and toSet was not accepted

LorenzoMartini · 2021-04-19T19:56:42Z

project/CirclePlugin.scala

-    }.value
+    test := (test andFinally Def.taskDyn {
+      copyTestReportsToCircle
+    }).value


Again, sbt got stricter. and the fancy tuple plugin stuff removed. This is the equivalent of the above

LorenzoMartini · 2021-04-20T07:33:09Z

project/plugins.sbt

The upstream pr changed the version of avro compiler to 1.8.2 but we're already ahead of it

Migrate sbt-launcher URL to download one for sbt 1.x. Update plugins versions where required by sbt update. Change sbt version to be used to latest released at the moment, 1.3.13 Adjust build settings according to plugins and sbt changes. Migration to sbt 1.x: 1. enhances dev experience in development 2. updates build plugins to bring there new features/to fix bugs in them 3. enhances build performance on sbt side 4. eases movement to Scala 3 / dotty No. All existing tests passed, both on Jenkins and via Github Actions, also manually for Scala 2.13 profile. Closes apache#29286 from gemelen/feature/sbt-1.x. Authored-by: Denis Pyshev <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>

rshkv · 2021-04-20T10:03:28Z

project/SparkBuild.scala

-  lazy val sparkGenjavadocSettings: Seq[sbt.Def.Setting[_]] = Seq(
-    libraryDependencies += compilerPlugin(
-      "com.typesafe.genjavadoc" %% "genjavadoc-plugin" % unidocGenjavadocVersion.value cross CrossVersion.full),
+  lazy val sparkGenjavadocSettings: Seq[sbt.Def.Setting[_]] = GenJavadocPlugin.projectSettings ++ Seq(


Looks like this is why the unidoc check started failing.

rshkv · 2021-04-20T10:03:44Z

project/SparkBuild.scala

-      "com.typesafe.genjavadoc" %% "genjavadoc-plugin" % unidocGenjavadocVersion.value cross CrossVersion.full),
+  lazy val sparkGenjavadocSettings: Seq[sbt.Def.Setting[_]] = GenJavadocPlugin.projectSettings ++ Seq(
    scalacOptions ++= Seq(
-      "-P:genjavadoc:out=" + (target.value / "java"),


Or this actually

rshkv · 2021-04-20T10:10:03Z

project/SparkBuild.scala

-      analysis.infos.allInfos.foreach { case (k, i) =>
-        i.reportedProblems foreach { p =>
-          val deprecation = p.message.contains("is deprecated")
+      analysis.asInstanceOf[sbt.internal.inc.Analysis].infos.allInfos.foreach { case (k, i) =>
+        i.getReportedProblems foreach { p =>
+          val deprecation = p.message.contains("deprecated")


(This previously was is deprecated in our fork. But checking for deprecated only should give you a superset.)

LorenzoMartini mentioned this pull request Apr 19, 2021

[SPARK-21708][BUILD] Migrate build to sbt 1.x #752

Closed

LorenzoMartini force-pushed the lmartini/sbt-1.x branch from 11da97d to 05fe273 Compare April 19, 2021 19:49

LorenzoMartini commented Apr 19, 2021

View reviewed changes

.circleci/config.yml Outdated

Copy link

Author

LorenzoMartini Apr 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This had to be removed because it picks up older build/sbt and doesn't get the updated versions

LorenzoMartini commented Apr 19, 2021

View reviewed changes

project/SparkBuild.scala Outdated

Copy link

Author

LorenzoMartini Apr 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sbt got stricter and toSet was not accepted

rshkv reacted with thumbs up emoji

LorenzoMartini force-pushed the lmartini/sbt-1.x branch from 6b73877 to 94b4feb Compare April 19, 2021 19:55

LorenzoMartini commented Apr 19, 2021

View reviewed changes

LorenzoMartini force-pushed the lmartini/sbt-1.x branch from f4de666 to fd66f3c Compare April 20, 2021 07:32

LorenzoMartini commented Apr 20, 2021

View reviewed changes

project/plugins.sbt Outdated

Copy link

Author

LorenzoMartini Apr 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The upstream pr changed the version of avro compiler to 1.8.2 but we're already ahead of it

LorenzoMartini force-pushed the lmartini/sbt-1.x branch from 465e437 to e45a2f8 Compare April 20, 2021 09:12

rshkv reviewed Apr 20, 2021

View reviewed changes

rshkv approved these changes Apr 20, 2021

View reviewed changes

LorenzoMartini mentioned this pull request Apr 20, 2021

Re-enable unidoc for sbt build #760

Open

LorenzoMartini merged commit f779ffb into master Apr 20, 2021

LorenzoMartini deleted the lmartini/sbt-1.x branch April 20, 2021 10:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-21708][BUILD] Migrate build to sbt 1.x#759

[SPARK-21708][BUILD] Migrate build to sbt 1.x#759
LorenzoMartini merged 1 commit intomasterfrom
lmartini/sbt-1.x

LorenzoMartini commented Apr 19, 2021 •

edited

Loading

Uh oh!

LorenzoMartini Apr 19, 2021

Uh oh!

LorenzoMartini Apr 19, 2021

Uh oh!

LorenzoMartini Apr 19, 2021

Uh oh!

LorenzoMartini Apr 20, 2021

Uh oh!

LorenzoMartini Apr 19, 2021

Uh oh!

LorenzoMartini Apr 19, 2021

Uh oh!

LorenzoMartini Apr 19, 2021

Uh oh!

LorenzoMartini Apr 20, 2021

Uh oh!

rshkv Apr 20, 2021

Uh oh!

rshkv Apr 20, 2021

Uh oh!

rshkv Apr 20, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

LorenzoMartini commented Apr 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Original PR description

Upstream SPARK-XXXXX ticket and PR link (if not applicable, explain)

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

LorenzoMartini commented Apr 19, 2021 •

edited

Loading